Refactor reports/metrics, add JSON and markdown report #600
Conversation
|
I updated the PR to make clippy happy. Although these are not files touched by the PR, it looks like newer Rust versions brought in new lints that now fail. |
ctron
changed the title
|
I updated the PR to support markdown as well. It should be pretty close to the HTML report, but without the charts. |
|
This looks great from a read through on my phone. I’m traveling internationally but will find time to test it and get it merged. In a follow up PR, it would be great to update the book documentation too. |
Cool. Take your time. I can update the PR over time. I am currently also trying to add the baseline feature. We can split that off, but it's based on the other changes anyway. |
|
does this mean a new release as well ? nudge nudge |
|
I pushed the change for baseline comparison too. Of course we can split this off. Looks like this:
|
Please split this off -- I'd like to get the first bits committed first. |
ctron
force-pushed
the
feature/reports_2
branch
2 times, most recently
from
38669ea
to
26ed40c
Compare
ctron
force-pushed
the
feature/reports_2
branch
from
26ed40c
to
ef178bd
Compare
I split it off (#602) and rebased the PR. |
|
I'm finally starting to test this now. We need to improve the documentation, as it's non-obvious what format of report files are supported. Further, it feels like there should be some sort of validation? For example, if you accidentally type |
There was a problem hiding this comment.
Overall this is fantastic, thanks for the contribution! However, there are a few small issues to resolve, see the earlier comments. In addition, it would be very helpful to:
- Add an entry to CHANGELOG.md
- Update affected documentation, including:
src/docs/goose-book/src/getting-started/running.md(https://book.goose.rs/getting-started/running.html),src/docs/goose-book/src/getting-started/runtime-options.md(https://book.goose.rs/getting-started/runtime-options.html),src/docs/goose-book/src/getting-started/common.md(https://book.goose.rs/getting-started/common.html#writing-an-html-formatted-report),src/docs/goose-book/src/getting-started/metrics.md(https://book.goose.rs/getting-started/metrics.html),src/docs/goose-book/src/config/defaults.md(https://book.goose.rs/config/defaults.html)
|
Updated the PR, and the docs as well. |
ctron
force-pushed
the
feature/reports_2
branch
from
3168b25
to
3157269
Compare
ctron
force-pushed
the
feature/reports_2
branch
from
3157269
to
19dad6f
Compare
I see, we're running with |
|
We can easily fix the gaggle issue in a follow-up, I've confirmed all other tests run correctly and without errors. This new functionality is greatly appreciated, thanks for the contribution! |
This PR refactors the code of the metrics and report module a bit. It extracts the common part into a common module, allowing to prepare the data consistently and then rendering it out into HTML, and also serializing the data into JSON.
For that the
--report-fileargument has been changed into aVec<String>to allow providing more than one file. It uses the file extension as an indicator of which file type should be created.In addition, this should make it easier to implement a markdown report too.